feat(drivers): pick a version, and see when your own copy has fallen behind#672
Conversation
…behind
Two gaps, both about an operator not being told something.
A local override was skipped entirely by EnrichCatalog, so UpstreamVersion and
UpdateAvailable were never computed for it. An override shadows the channel, so
running your own copy meant never finding out a newer version existed.
It is now told. Resolution is untouched: the local file keeps winning, and the
UI says so rather than offering an Update button that would install a version
the override goes on shadowing. Being told is not being changed.
UpdateAvailable is also gated on both versions being real semver.
compareSemver falls back to comparing strings, so an override versioned
"local" or not versioned at all would have announced an update on alphabetical
luck. RepositoryID stays empty for a local file, because naming a repository
there would read as provenance it does not have.
The second gap: GET /api/device_repository/drivers/{id}/versions has always
existed and no UI called it. Rollback steps back exactly one version, so a
specific older one was unreachable even though the channel keeps every version
it has ever signed -- 110 of them today. Each managed driver now has a Versions
list showing what is downloaded, what the channel offers, and which is live.
Activating something already on disk needs no network; anything else is fetched
first, and the labels say which is happening.
The panel builds DOM rather than assembling HTML. Version strings come from a
signed manifest, but a signed manifest is still remote input.
Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 721c65ea56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| available.forEach(function (v) { | ||
| var version = typeof v === "string" ? v : (v && v.version); | ||
| if (!version || installedVersions[version]) return; | ||
| rows.push({version: version, active: false, local: false}); |
There was a problem hiding this comment.
Read the version from the nested candidate
When a version exists only in the signed repository, the versions endpoint returns each available item as a VersionCandidate shaped like {repository_id, driver: {version, ...}}, not as an object with a top-level version (the existing update-badge.js consumer follows this shape). Consequently v.version is undefined and every channel-only candidate is skipped, leaving bundled drivers with a blank panel and preventing the advertised selection of older remote versions.
Useful? React with 👍 / 👎.
| var payload = row.local | ||
| ? {version: row.version, sha256: row.sha256 || ""} | ||
| : {version: row.version}; |
There was a problem hiding this comment.
Include the repository when fetching a version
When the operator selects a version that is not downloaded, this payload contains only version, but handleDeviceRepositoryInstall rejects requests unless either repository_id or channel is present. Thus every “Fetch and activate” request will return HTTP 400 even after the available candidates are rendered; preserve the candidate's repository_id in the row and send it with the install request.
Useful? React with 👍 / 👎.
* fix(drivers): make version switching work, and reversible Four faults, each hidden behind the one in front of it. The signed channel could not install anything at all. parseCatalogEntry only matched a DRIVER table written inline, but tools/ftw_repository.py assigns it from a local -- twice, so a driver cannot overwrite the identity it was signed under. Every signed artifact therefore parsed as empty metadata and validateLuaArtifact refused it with "driver metadata id/version @, want ferroamp@1.1.1". Update failed the same way; nobody noticed because nothing reached that far. The Versions panel from #672 rendered zero rows. GET /versions answers with VersionCandidate -- {repository_id, driver:{version}, installed?} -- and the picker read .version off the outer object, so the guard dropped every row. Its install call also omitted repository_id, which the endpoint requires. The test that was meant to cover this matched the source with a regex, so it passed against an empty panel. Rolling back the first managed install was refused: PreviousInstalledPath is empty, and the bundled driver is not an install to step back to. That is the common case -- installing over the bundled driver is the first thing anyone does when trying a new one. Rollback now removes the managed entry, which is what makes core resolve the bundled copy again, and the restart points config at the bundled path rather than the symlink it just deleted. The row itself said BUNDLED · V1.0.0 beside three buttons, where Rollback was a subset of Versions and bundled/managed/local is our vocabulary. Every driver runs locally; what matters is which version runs and what you can switch to. So the row now reads "v1.0.0 · official, shipped with this build · verified on hardware", the catalog's verification_status finally appears where updates are chosen, and one Versions list replaces Rollback. After a switch, Undo puts the old version back -- activating it when it is retained, rolling back when it is the bundled copy -- and re-arms the switch so trying again is one click. The web tests now drive the real code with the payload the API really returns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(drivers): a standing way back to the bundled driver, not a transient undo An external review found that removing the Rollback button left no lasting way back. /versions lists signed and retained artifacts; the bundled copy is neither, so once a channel version was installed the only path back lived in an Undo button that vanished when the panel closed. Worse, the rollback-to-bundled path added in the previous commit could take a driver offline. Not every driver the channel offers is bundled, and pointing the restart at a file that is not there makes Registry.Restart remove the working instance before Add fails. The recovery path then called Rollback a second time, which had already deactivated the only active row and had nothing left to restore -- the "sql: no rows in result set" seen while testing. So going back to the bundled copy is now its own operation rather than a step inside Rollback. UseBundled checks the bundled file exists before deactivating anything, and returns the artifact it replaced so the caller can put it back if the bundled driver fails to start. Rollback returns to refusing a first install, which keeps its own recovery path intact. The version list carries a standing "the copy shipped with this build" row, so the way back survives closing the panel. Also from the review: the dedupe key was the version alone, which dropped one of two repositories publishing the same version with different hashes -- the hash is the artifact's identity, so it is the key. A row fetched from the channel now records that it is on disk, so a second attempt activates it instead of downloading again and failing offline. Undo picks its route from what was running rather than from a version number, since a managed artifact of the same version can sit on disk and differ in whether it may control anything. The summary line above the panel is rebuilt from the catalog after a switch rather than edited in place. Which version runs, its provenance, how well tested it is and whether it may control anything are all properties of the artifact now running; the telemetry-only badge in particular differs between versions of one driver, and it was stale in both directions. The DOM stub in the tests answered null from querySelector, which let a test pass while the code it covered did nothing. It resolves properly now, verified by breaking markRunning and watching the test fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Two gaps, both about an operator not being told something.
A local override never heard about updates
EnrichCatalogskipped local entries outright:So
UpstreamVersionandUpdateAvailablewere never computed for a driver you supply yourself. An override shadows the channel, which means running your own copy meant never finding out a newer version existed.It is now told. Resolution is untouched — the local file keeps winning, and the UI says so rather than offering an Update button that would install a version the override goes on shadowing:
Being told is not being changed. You keep your copy until you decide otherwise.
Two details that matter:
UpdateAvailableis gated on both versions being real semver.compareSemverfalls back to comparing strings, so an override versioned"local"— or not versioned at all — would have announced an update on alphabetical luck.RepositoryIDstays empty for a local file. Naming a repository there would read as provenance it does not have, and an existing test rightly guards that.The version list existed and nothing used it
GET /api/device_repository/drivers/{id}/versionshas been there all along with no caller. Rollback steps back exactly one version, so a specific older one was unreachable — even though the channel keeps every version it has ever signed, 110 of them today. That history is the whole point of an immutable channel and none of it was reachable.Each managed driver now has a Versions… list:
v1.4.0 · running,v1.3.1→ Activate (no network)v2.0.0→ Fetch and activateThe labels distinguish the two so it is clear why one is instant. The running version carries no button.
The panel builds DOM rather than assembling HTML. Version strings come from a signed manifest, but a signed manifest is still remote input.
Tests
web/driver-versions.test.mjspins both properties, including the one that pulls against the other: an override must be told, and must not be quietly replaced.Go:
internal/driverrepocovers the override being told what the channel has, an older override genuinely offering an update, and an unversioned one making no claim either way.Web 117 passed ·
make verifyclean.🤖 Generated with Claude Code